Skip to content

Support CMake versions 3.8-3.12.4 - #314

Closed
fultoncjb wants to merge 1 commit into
lava:masterfrom
fultoncjb:ExtendCmake
Closed

fultoncjb wants to merge 1 commit into
lava:masterfrom
fultoncjb:ExtendCmake

Conversation

@fultoncjb

Copy link
Copy Markdown

The repository currently does not support CMake versions 3.8 to 3.12.4 because the FindPython3.cmake module was not added until 3.12.4 and the minimum CMake version for the project is set to 3.8. This PR adds the necessary CMake code to still correctly compile and link to the relevant Python libraries.

@lava

lava commented Nov 3, 2026

Copy link
Copy Markdown
Owner

Thanks for working on support for older CMake versions, and sorry for the long wait. Making the build work on more toolchains is welcome, but I can't merge this PR as it stands. It contains two separate changes:

1. FindPythonLibs fallback for CMake < 3.12.4. I'm closing this part without prejudice. The fallback has some problems:

  • FindPythonLibs doesn't accept COMPONENTS, so Interpreter Development NumPy is silently ignored. Nothing finds the numpy include directory, and WITHOUT_NUMPY isn't defined either. On most systems, numpy/arrayobject.h is not under the Python include directory, so the build would fail when it compiles the header.
  • The package isn't REQUIRED, so a missing Python shows up later as a confusing compile error instead of a clear configure-time error.
  • The version cutoff doesn't match what the else() branch needs. FindPython3 exists from CMake 3.12, but its NumPy component needs 3.14 and the Python3::Module target needs 3.15. CMake 3.12–3.14 would still fail.

If you'd like to try again, a resubmission is welcome. A version that picks the fallback based on the features actually used, finds the numpy include path (for example by asking the Python interpreter for numpy.get_include()) or else defines WITHOUT_NUMPY, and marks Python as REQUIRED would be a good starting point.

2. Reindentation of the rest of CMakeLists.txt. I'm closing this part as well. It's a whitespace-only change unrelated to the fix, and it makes the functional change harder to review. In any resubmission, please keep the existing 2-space indentation and limit the diff to the lines that need to change.

Thanks again for the contribution!


🤖 This review was made by the project's automated governance process. See the README for details.

@lava lava closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants